lib.escapeRegex: fix with boost::regex implementation#271245
Closed
9999years wants to merge 1 commit intoNixOS:masterfrom
Closed
lib.escapeRegex: fix with boost::regex implementation#2712459999years wants to merge 1 commit intoNixOS:masterfrom
boost::regex implementation#2712459999years wants to merge 1 commit intoNixOS:masterfrom
Conversation
NixOS/nix#7762 switched `nix` to use the `boost::regex` implementation, which expects that all literal braces are escaped. That is, the regex `\{crate}` no longer parses. We modify `lib.escapeRegex` to accommodate this change. Fortunately, the old regex implementation doesn't mind if closing braces are escaped as well, so this is backwards compatible and we don't need to worry about version-gating it. Without this patch: Old regex implementation: $ nix repl nixpkgs Welcome to Nix 2.18.1. Type :? for help. Loading installable 'flake:nixpkgs#'... Added 5 variables. nix-repl> builtins.match (lib.escapeRegex "{}") "{}" [ ] $ ~/nix/result/bin/nix repl nixpkgs Welcome to Nix 2.20.0pre20231130_dirty. Type :? for help. `boost::regex` implementation: $ ~/nix/result/bin/nix repl nixpkgs Welcome to Nix 2.20.0pre20231130_dirty. Type :? for help. Loading installable 'flake:nixpkgs#'... Added 5 variables. nix-repl> builtins.match (lib.escapeRegex "{}") "{}" error: … while calling the 'match' builtin at «string»:1:1: 1| builtins.match (lib.escapeRegex "{}") "{}" | ^ error: invalid regular expression '\{}'
Member
|
This is a Nix regression that should just get reverted. Nixpkgs is the best test suite we have for Nix, if there's a regression for Nixpkgs there will also be regressions for third-parties. I submitted a revert PR here: NixOS/nix#9508 |
Contributor
Author
|
But the fix is back-compatible -- why not just roll forward? |
Member
|
@9999years The problem is that Nix broke backwards compatibility. Older versions of Nixpkgs should always work with newer Nix versions. |
Member
|
Revert in NixOS/nix#9508 was merged, it also includes your report as a test case, so this shouldn't be a problem going forward :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NixOS/nix#7762 switched
nixto use theboost::regeximplementation, which expects that all literal braces are escaped. That is, the regex\{crate}no longer parses. We modifylib.escapeRegexto accommodate this change. Fortunately, the old regex implementation doesn't mind if closing braces are escaped as well, so this is backwards compatible and we don't need to worry about version-gating it.Here's how
nixworks without this patch:Old regex implementation:
boost::regeximplementation:Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Priorities
Add a 👍 reaction to pull requests you find important.